clarify CID length requirements for VN packets #4187

marten-seemann · 2020-10-08T01:50:03Z

…uirements

draft-ietf-quic-invariants.md

martinthomson · 2020-10-08T03:01:27Z

draft-ietf-quic-transport.md

-limit, Version Negotiation packets could carry Connection IDs that are longer
-than 20 bytes.
+Future versions of QUIC may have different requirements for the lengths of
+connection IDs larger than version 1. In particular, connection IDs might not


Edit: old suggestion

connection IDs. Other versions might allow connection IDs to be longer. Other versions might permit shorter or longer connection IDs to be included in packets sent by a client to open a connection. A server MUST send a Version Negotiation packet in response to a packet containing a connection ID smaller than 8 bytes or larger than 20 bytes.

@martinthomson Isn't the second sentence (i.e., "Other versions might allow connection IDs to be longer.") excessive? The next sentence covers that, I think.

kazuho · 2020-10-08T03:13:30Z

draft-ietf-quic-transport.md

+connection IDs larger than version 1. In particular, connection IDs might not
+be required to have a minimum lengths, and might be longer than 20 bytes.  A
+server therefore MUST respond with a Version Negotiation packets for all
+lengths of connection IDs allowed by the layout defined here.


I'm not sure if "MUST send" is the correct phrase here. This would be a contradiction to what we already have in section 5.2.2, quote: if a server receives a packet that indicates an unsupported version but is large enough to initiate a new connection for any supported version, the server SHOULD send a Version Negotiation packet as described in Section 6.1.

I think what we need to say here is that the CID length rules do not apply to other protocol versions, and no more.

PS. Therefore I think that this paragraph can be as simple as something like: Note that version-specific rules for QUIC packets (e.g., limits on connection ID lengths) SHOULD NOT influence the decision of whether or not to send Version Negotiation packets, because they might change between the versions.

That works for me also. (I wasn't sure about the "MUST" for that reason, but I couldn't find the right words.) A "MUST NOT" would work for your suggestion:

Version-specific rules for QUIC packets MUST NOT influence a server decision about whether to send a Version Negotiation packet.

I don't think we should say a server MUST send a VN. Makes it sound like it's a protocol violation if I don't respond to a flood of packets, each with their own VN packet. For instance, if MsQuic receives a burst of packets from an endpoint, it generally sends a single VN packet in response.

martinthomson · 2020-10-08T04:55:43Z

draft-ietf-quic-transport.md

+Future versions of QUIC may have different requirements for the lengths of
+connection IDs larger than version 1. In particular, connection IDs might not
+be required to have a minimum lengths, and might be longer than 20 bytes.  A
+server therefore MUST respond with a Version Negotiation packets for all
+lengths of connection IDs allowed by the layout defined here.


Suggested change

Future versions of QUIC may have different requirements for the lengths of

connection IDs larger than version 1. In particular, connection IDs might not

be required to have a minimum lengths, and might be longer than 20 bytes. A

server therefore MUST respond with a Version Negotiation packets for all

lengths of connection IDs allowed by the layout defined here.

Future versions of QUIC may have different requirements for the lengths of

connection IDs. Version-specific rules for QUIC packets MUST NOT influence a

server decision about whether to send a Version Negotiation packet.

larseggert · 2020-10-08T07:47:48Z

Could we also maybe add a pointer to the requirement that a DCID in an Initial must be eight bytes to Section 17.2? Because at the moment, it's buried in Section 7.2, where I didn't find it.

MikeBishop

I think this is a good improvement, but some wordsmithing could still be done.

MikeBishop · 2020-10-08T15:33:18Z

draft-ietf-quic-transport.md

+connection IDs larger than version 1. In particular, connection IDs might not
+be required to have a minimum lengths, and might be longer than 20 bytes.  A


Suggested change

connection IDs larger than version 1. In particular, connection IDs might not

be required to have a minimum lengths, and might be longer than 20 bytes. A

connection IDs. In particular, connection IDs might have a smaller minimum

length or a greater maximum length. A

MikeBishop · 2020-10-08T15:35:44Z

draft-ietf-quic-transport.md

+connection IDs larger than version 1. In particular, connection IDs might not
+be required to have a minimum lengths, and might be longer than 20 bytes.  A
+server therefore MUST respond with a Version Negotiation packets for all
+lengths of connection IDs allowed by the layout defined here.


Suggested change

lengths of connection IDs allowed by the layout defined here.

lengths of connection IDs allowed by the layout defined in Section 5.1

of {{QUIC-INVARIANTS}}.

martinthomson · 2020-10-11T22:20:23Z

draft-ietf-quic-invariants.md

@@ -316,6 +316,9 @@ connection IDs gives clients some assurance that the server received the packet
 and that the Version Negotiation packet was not generated by an off-path
 attacker.

+Version-specific rules for QUIC packets MUST NOT influence a server decision


The text in the transport doc is fine; this is, as David says, a little sketchy without the minimum length constraint. A prefix of "other than the minimum packet size, ..." might work, I guess.

Or scope the MUST NOT specifically to CID interpretation.

Yes, I think this proposal is unclear on what the "MUST NOT" is for.

gloinul

I think the CID specific aspect is fine. It is unclear what other aspects that are specified by the protocol and are not invariant here.

gloinul · 2020-10-15T12:48:45Z

draft-ietf-quic-invariants.md

@@ -316,6 +316,9 @@ connection IDs gives clients some assurance that the server received the packet
 and that the Version Negotiation packet was not generated by an off-path
 attacker.

+Version-specific rules for QUIC packets MUST NOT influence a server decision


Yes, I think this proposal is unclear on what the "MUST NOT" is for.

gloinul · 2020-10-15T12:50:30Z

draft-ietf-quic-transport.md

-than 20 bytes.
+Future versions of QUIC may have different requirements for the lengths of
+connection IDs. In particular, connection IDs might have a smaller minimum
+length or a greater maximum length.  Version-specific rules for QUIC packets


With the context of the first new sentence the second sentence is clearly apply to CIDs, but it indicates that there might be other aspects. What aspects are not invariant here?

martinthomson · 2020-10-15T23:29:39Z

I've taken this change manually, after reverting the change to the invariants doc. After looking at this more closely, the invariants doc does not include specifics of how to generate a Version Negotiation packet and this takes an awkward step in that direction.

Rather than start to import the rules into invariants piecemeal like this, I think that it is cleaner to leave the rules for generating a Version Negotiation packet as version-specific, which is already clearly the case.

martinthomson · 2020-10-15T23:32:09Z

Addendum: I missed the bit about this being a blanket "version-specific rules for packets". I've changed this to "version-specific rules for connection IDs", to reflect the context better. And to avoid tripping up on the packet size rules in version 1.

marten-seemann added 2 commits October 8, 2020 08:48

add text about CID lengths to the invariants

9fc4a11

clarify that future QUIC versions might have different CID length req…

ec17412

…uirements

marten-seemann requested review from janaiyengar and martinthomson as code owners October 8, 2020 01:50

nibanks approved these changes Oct 8, 2020

View reviewed changes

martinthomson reviewed Oct 8, 2020

View reviewed changes

kazuho reviewed Oct 8, 2020

View reviewed changes

martinthomson reviewed Oct 8, 2020

View reviewed changes

MikeBishop reviewed Oct 8, 2020

View reviewed changes

marten-seemann added 2 commits October 9, 2020 11:09

use @martinthomson's suggestion for the invariants

05cc5d9

apply @MartinThompson's and @MikeBishop's suggestions

3fec711

kazuho approved these changes Oct 9, 2020

View reviewed changes

nibanks approved these changes Oct 9, 2020

View reviewed changes

DavidSchinazi reviewed Oct 9, 2020

View reviewed changes

gloinul reviewed Oct 15, 2020

View reviewed changes

martinthomson merged commit 3fec711 into quicwg:master Oct 15, 2020

martinthomson added -transport editorial An issue that does not affect the design of the protocol; does not require consensus. labels Oct 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clarify CID length requirements for VN packets #4187

clarify CID length requirements for VN packets #4187

marten-seemann commented Oct 8, 2020

martinthomson Oct 8, 2020 •

edited

Loading

kazuho Oct 8, 2020

kazuho Oct 8, 2020 •

edited

Loading

kazuho Oct 8, 2020

martinthomson Oct 8, 2020

nibanks Oct 8, 2020

martinthomson Oct 8, 2020

larseggert commented Oct 8, 2020

MikeBishop left a comment

MikeBishop Oct 8, 2020

MikeBishop Oct 8, 2020

This comment was marked as resolved.

martinthomson Oct 11, 2020

MikeBishop Oct 12, 2020

gloinul Oct 15, 2020

gloinul left a comment

gloinul Oct 15, 2020

gloinul Oct 15, 2020

martinthomson commented Oct 15, 2020

martinthomson commented Oct 15, 2020

		connection IDs larger than version 1. In particular, connection IDs might not
		be required to have a minimum lengths, and might be longer than 20 bytes. A

	lengths of connection IDs allowed by the layout defined here.
	lengths of connection IDs allowed by the layout defined in Section 5.1
	of {{QUIC-INVARIANTS}}.

clarify CID length requirements for VN packets #4187

clarify CID length requirements for VN packets #4187

Conversation

marten-seemann commented Oct 8, 2020

martinthomson Oct 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazuho Oct 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larseggert commented Oct 8, 2020

MikeBishop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gloinul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinthomson commented Oct 15, 2020

martinthomson commented Oct 15, 2020

martinthomson Oct 8, 2020 •

edited

Loading

kazuho Oct 8, 2020 •

edited

Loading